-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Swapping PyCrypto for pyOpenSSL. #1338
Conversation
To "confirm" this implementation matches our current one I did the following: >>> import json
>>> from oauth2client.client import GoogleCredentials
>>> from OpenSSL import crypto
>>> from gcloud import credentials
>>> creds = GoogleCredentials.get_application_default() # Env. var. -> path to JSON key
>>> pkey = crypto.load_privatekey(crypto.FILETYPE_PEM, creds._private_key_pkcs8_text)
>>> data = b'foo'
>>> new_signed_bytes = crypto.sign(pkey, data, 'SHA256')
>>> curr_signed_bytes = credentials._get_signature_bytes(creds, data)
>>> new_signed_bytes == curr_signed_bytes
True |
H/T to @tseaver for pointing out that |
@@ -20,6 +20,8 @@ class _Monkey(object): | |||
|
|||
def __init__(self, module, **kw): | |||
self.module = module | |||
if len(kw) == 0: # pragma: NO COVER | |||
raise ValueError('_Monkey was used with nothing to monkey-patch') |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This was done because PyCrypto does not install easily on Windows. pyOpenSSL is managed by PyCA (the Python crypto authority) and has a mature release process. This change was influenced by discussions about googleapis#1009.
ae525ed
to
948cccf
Compare
@jonparrott Is |
I don't think so; only |
OK. It may be best to take the |
Sounds reasonable. |
I'd really hate to have our codebase cluttered with both the current |
Maybe now is the time to bite the bullet and get both implementations into Then the question becomes
|
I really don't want to introduce imperative platform-based variations in the requirements (breaking wheel generation, for instance). Is there a PEP 508-supported way to detect GAE? |
No. |
@jonparrott Wrote https://github.com/jonparrott/Darth-Vendor which probably makes him the foremost expert on packaging for GAE 😀 |
@jonparrott After some digging I realized this can be done in pure Python using |
@dhermes if they have any native components, then yes. Let me verify. |
@dhermes those should be fine. Go for it. 👍 |
Good deal. Thanks for doing it for me (I know I could've RTFM instead of wasting your time). |
Considering how much of your time I've monopolized elsewhere, I'd say I owe you. |
As I was starting to do the @jonparrott What does darth vendor do if a package can't be installed / imported in GAE? |
@dhermes nothing. pip handles the installation, not the vendor tool. Pip will happily stage a binary package into the |
OK. I suppose we could |
Would the |
What did you have in mind? i.e. Try and except what import? How would it help GAE? |
I'm responding to:
|
I meant |
|
@jonparrott Do you think it's fine to just leave |
I think you're in the clear as long as you don't ever try to use the library within GAE code. |
@tseaver GAE is "no longer" a blocker for using |
I thought GAE was the only reason to keep the |
Nope its Windows. Check out the description of this PR. |
I just scared myself into thinking that it'd be a blocker for GAE but forgot that GAE and GCE had custom credentials types |
Oops, I misspoke: I meant I thought the old codepath ( |
Got it. All good to merge? (PS I am planning on pushing this upstream to |
LGTM. I had lost track of the fact that you already dropped the old codepath. |
Swapping PyCrypto for pyOpenSSL.
This was done because
PyCrypto
does not install easily on Windows.pyOpenSSL
is managed by PyCA (the Python crypto authority) and has a mature release process.This change was influenced by discussions about #1009.